Skip to content

feat: Docs for client secret rotation#2424

Open
SamuelDev wants to merge 9 commits into
ory:masterfrom
SamuelDev:client-secret-rotation-docs
Open

feat: Docs for client secret rotation#2424
SamuelDev wants to merge 9 commits into
ory:masterfrom
SamuelDev:client-secret-rotation-docs

Conversation

@SamuelDev

Copy link
Copy Markdown

Related Issue or Design Document

Docs updates for client secret rotation

Checklist

  • I have read the contributing guidelines and signed the CLA.
  • I have referenced an issue containing the design document if my change introduces a new feature.
  • I have read the security policy.
  • I confirm that this pull request does not address a security vulnerability.
    If this pull request addresses a security vulnerability,
    I confirm that I got approval (please contact security@ory.com) from the maintainers to push the changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added the necessary documentation within the code base (if appropriate).

Further comments

@SamuelDev

Copy link
Copy Markdown
Author

Javascript integration test failure seems like a CI issue and unrelated to these changes

@vinckr vinckr added the upstream Issue is caused by an upstream dependency. label Jan 23, 2026
@alnr alnr added the area/docs This issue affects Ory's documentation. label Feb 10, 2026
@alnr

alnr commented Feb 11, 2026

Copy link
Copy Markdown
Contributor

@wassimoo @unatasha8 would you kindly take a look at this pls?

@vinckr vinckr force-pushed the client-secret-rotation-docs branch from 19afb45 to 29299a0 Compare February 11, 2026 23:14

@vinckr vinckr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm :) fixed some layout issues

Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated

@unatasha8 unatasha8 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, keep to singular for 'secret' if the process is one at a time. Pick one to use client(s), client service(s), service(s), application(s) etc. I tried to change all to client services.

Apply the terminology suggestions from the review: singular "secret", and
"client service" used consistently.

Also fix the rotation endpoint path. All four curl examples used
/admin/clients/{id}/secret/rotate, but the implemented route is
/admin/clients/{id}/secrets/rotate, so every example returned 404.

Answer the two open review questions in the text: say where to store the new
secret, and confirm that secrets do not expire on their own.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@alnr

alnr commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Pushed 056df20 to address @unatasha8's review. Summary of what changed, including three places where I did not apply a suggestion literally — flagging those so you can push back.

Applied

  • Singular "secret" throughout, and "client service(s)" used consistently, per the general note.
  • "Clear rotated secrets" → "Remove old secret"; "cleanup" wording removed everywhere.
  • The two admonitions and the workflow example comments now use the agreed terminology.

Bug found while editing

All four curl examples used /admin/clients/{id}/secret/rotate (singular). The implemented route is /admin/clients/{id}/secrets/rotate (plural) — see client/handler.go and the OpenAPI spec in ory-corp/cloud#10632. Every example on this page would have returned 404. Fixed.

Answers to the open questions

  • "Where should this be saved?" (line 160) — the text now says Ory shows the value only once and to store it in the secret store the client services read from (a secrets manager or an encrypted environment configuration).
  • "'Secrets remain valid indefinitely' — are you sure? Don't they have a validity date?" (line 260) — confirmed correct. Rotated secrets are stored in the rotated_secrets column with no expiry field and no TTL; they stay valid until explicitly deleted. That is exactly why the warning admonition exists. This is unlike the SAML valid_to attribute you mentioned, which is a different feature.

Deviations from the literal suggestions

  1. Ordering of "How secret rotation works" — the suggestion moves "Both old and new secrets authenticate" to step 3, after "Update your client services". That would imply the dual-validity window starts once you update your services. It actually starts the moment you rotate. I kept the dual-validity statement attached to step 1 and added your "test before removing" step as step 3:
    1. Generate a new secret for the client service. Both the old and the new secret authenticate from this point on.
    2. Update your client services to use the new secret.
    3. Test that the client services can authenticate with the new secret.
    4. Manually remove the old secret. Only the new secret authenticates from this point on.
  2. ##### Rotate OAuth2 client secret — left as ##### Rotate client secret. The ## heading directly above is already "Rotate OAuth2 client secret", so the suggestion would create two identical headings and an ambiguous #rotate-oauth2-client-secret-1 anchor.
  3. "servcies" in the step-3 suggestion is a typo; used "services".

Happy to take any of these the other way — say the word.

Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated
Comment thread docs/hydra/guides/oauth2-clients.mdx Outdated

@unatasha8 unatasha8 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See new comments

alnr and others added 2 commits July 14, 2026 11:03
…lementation

- Pluralize 'secrets' and use 'can be used to authenticate' wording per review
- Rename subheading to 'Rotate OAuth2 client secret'
- Document auth method restriction (client_secret_basic/client_secret_post),
  the five-secret retention limit, and that set/patch clears rotated secrets

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docs This issue affects Ory's documentation. upstream Issue is caused by an upstream dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants